-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Add french translation missing #20061
fix: Add french translation missing #20061
Conversation
remove translation for datePicker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contributon @aehanno . I would recommend splitting this PR into multiple separate PRs, as there are many unrelated changes here. Specifically:
- The additional French translations (completely valid)
- Fixing the untranslated strings, like "Home" (this is completely valid)
- using
gettext
instead oflazy_gettext
(this is probably ok, but should be addressed in a separate PR, perhaps fixing other simiar instances) - Translating variables (these won't work)
- Using
moment
to calculate thelastModified
value fronchanged_on
instead ofchanged_on_humanized
+ addinglast_run
(this is controversial, but we can probably have that discussion in a separate PR)
label: t('Sort by %s', label), | ||
label: t('Sort by %s', t(label)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look right; you can't translate variables like this (the original code looks correct)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR: #20080
lastModified={cellData.changed_on_humanized} | ||
lastModified={moment.utc(cellData.changed_on).fromNow()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be changed (@dpgaspar can comment in more detail, but IIRC, the changed_on
property needs to be constructed in the backend right now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed_on_humanized has no translation. I will be glade to keep 'changed_on_humanized' but what can I change so that the translation is done
I move the change on this PR : #20082
we can discuss about that on the new PR
lastModified={slice.changed_on_humanized} | ||
lastModified={moment.utc(slice.changed_on).fromNow()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
@@ -314,7 +314,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) { | |||
<Divider /> | |||
<div> | |||
<div className="section-title">{t('Actual time range')}</div> | |||
{validTimeRange && <div>{evalResponse}</div>} | |||
{validTimeRange && <div>{t(evalResponse)}</div>} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, you can't translate a variable (please see e.g. https://stackoverflow.com/questions/34579316/flask-babel-how-to-translate-variables for an explanation why this is the case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change has been move in this PR : #20080
We can discuss about translation of varaible in the ohter PR
Without the t() here, there is no translation, do you have an idea, where I can translate this
Hello @villebro, |
Hi @villebro Is there anything we can do to help move these PRs forward? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for adding these missing translations!
@mathieudruart it appears you need to fix some linting errors in the translation file: |
Codecov Report
@@ Coverage Diff @@
## master #20061 +/- ##
==========================================
- Coverage 66.55% 66.07% -0.49%
==========================================
Files 1692 1732 +40
Lines 64802 65886 +1084
Branches 6657 6753 +96
==========================================
+ Hits 43129 43532 +403
- Misses 19973 20636 +663
- Partials 1700 1718 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
hey @villebro, correction of workflow has been made, it's possible to launch it ? |
Hi @villebro it should be fixed, is it possible to rerun the workflows ? Thanks ! |
Hi @villebro do you know when this PR will be merged ? thank you ! |
Hey @mathieudruart , sorry for the delay; merged! |
(cherry picked from commit 944808a)
(cherry picked from commit 944808a)
(cherry picked from commit 944808a)
SUMMARY
Adding french translation
Add Information
Fixes #20060